fix(console,runner): render the approvals inbox against one ticking clock, and lint both packages (#2927) - #2930
Merged
Conversation
…lock, and lint both packages (#2927) `apps/console` and `packages/runner` had no `lint` script, so `turbo run lint` skipped them silently and their 17 ESLint errors had never been seen. Both now carry `"lint": "eslint ."`, and the `DEBT` list in check-lint-coverage.mjs is empty — 45/45 workspace packages are linted. Read one by one, the 17 were not one class of problem: 8x react-hooks/purity — real. The approvals inbox read `Date.now()` mid-render for every age tint, "5m ago" label and SLA chip. Render must be pure: the output depended on when React happened to render, so it disagreed with itself under StrictMode's double render and then froze — an inbox left open kept saying "just now" and an SLA countdown never counted down. The page now renders against a single `now` held in state and advanced once a minute (the finest granularity it displays), so render is a pure function of props+state and the figures tick. `sla_due_at` also goes through a parse guard now: a due date the backend sends in a shape `Date.parse` can't read rendered as "SLA NaNh left"; it renders nothing. 1x react-hooks/static-components — real. `StatusBadge` was declared inside `ApprovalsInboxPage`, so it was a brand-new component type every render and React remounted every status chip in the table on each re-render. Hoisted to module scope with the translated label passed as a prop. 6x react-hooks/static-components — false positives (3 settings pages, 3 in the runner's LayoutRenderer). All six render the result of `getIcon`/`getLazyIcon`, which memoises per name in a module-level cache: the reference is stable and nothing is created during render. They carry the same targeted disable + justification the repo already uses at a dozen icon-registry sites, and both resolvers now document the cache. Verified rather than assumed — typing into a settings field keeps focus and every character, so nothing was being reset. 2 minor — a dead `token` initializer on the auth-preflight path (read, not blind -deleted: no intended write was missing) and a `prefer-const`. Closes #2927. Refs #2911, #2923. Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2927. Refs #2911, #2923.
apps/consoleandpackages/runnerhad nolintscript, soturbo run lintskipped them silently and their 17 ESLint errors had never been seen. Both now carry"lint": "eslint .", and theDEBTlist inscripts/check-lint-coverage.mjsis empty — 45/45 workspace packages are linted.The issue framed these as "12 React-correctness bugs". Read one by one they are not one class of problem, and the count is smaller than it looked:
8x
react-hooks/purity— real, and the fix is a behaviour fixThe approvals inbox read
Date.now()mid-render for every age tint, "5m ago" label and SLA chip. Render must be pure: the output depended on when React happened to render, so it disagreed with itself under StrictMode's double render — and, more visibly, it froze. An inbox left open kept saying "just now"; an SLA countdown never counted down.The page now renders against a single
nowheld in state and advanced once a minute (the finest granularity anything here displays — "5m ago", "36h", "3d"). Render becomes a pure function of props+state and the figures actually tick. Note a mount-frozen constant would have been worse than the impure read, so the interval is part of the correct fix, not scope creep.sla_due_atnow goes through a parse guard as well: a due date in a shapeDate.parsecan't read used to render as "SLA NaNh left"; it now renders nothing.1x
react-hooks/static-components— realStatusBadgewas declared insideApprovalsInboxPage, so it was a brand-new component type on every render and React unmounted/remounted every status chip in the table on each re-render. Hoisted to module scope, translated label passed as a prop.6x
react-hooks/static-components— false positives3 in the console's settings pages, 3 in the runner's
LayoutRenderer. All six render the result ofgetIcon/getLazyIcon, which memoises per name in a module-level cache — the component reference is stable across renders and nothing is created during render. The rule cannot see through the call.They carry the same targeted
eslint-disable-next-line+ justification the repo already uses at a dozen icon-registry sites (MetricCard,NavigationRenderer,action-group, …), and both resolvers now document the cache so the next reader doesn't have to re-derive it.This is the part of the issue that needed checking rather than believing. The issue called these "the user-visible one … forms, where state loss is most noticeable", and asked whether known Settings flakiness traced to them. It does not: typing 17 characters into the Company → Legal name field keeps focus and every character (screenshot below). Nothing was being reset.
2 minor
auth-preflight.ts— deadtokeninitializer. Read rather than blind-deleted, as the issue asked: no intended write is missing; every path out of the try/catch either assigns or returns. Dropped the initializer and documented why.sdui-workbench-preview.tsx—prefer-const.Verification
apps/console0 errors,packages/runner0 errors (were 14 / 3).node scripts/check-lint-coverage.mjs→45/45 packages linted, 0 with outstanding errors.prefer-constinto console and aDate.now()-in-render into runner failsturbo run lintin both (Failed: @object-ui/console#lint,@object-ui/runner#lint); reverting returns both to0 errors.tsc --noEmitclean in both packages;vitest91/91 in console, runner green.SLA 已超期 2din red,SLA 剩余 30hmuted, and nothing at all for an unparsable due date (the new guard);HTTP request failed, a duplicate-React-key warning) reproduces identically withApprovalsInboxPage.tsxswapped back to itsmainversion, so it is not from this change — filed separately.🤖 Generated with Claude Code